Skip to content
This repository has been archived by the owner on Nov 19, 2024. It is now read-only.

Add service for storage of tokens in auth properties #100

Merged
merged 6 commits into from
May 2, 2024

Conversation

josephdecock
Copy link
Member

This is pretty complex logic, because we can have per-scheme and/or per-resource tokens, and we need to store additional metadata as well. By exposing this as a service, we can reuse the logic elsewhere (I needed it in the BFF to use its session store as a token store).

This is pretty complex logic, because we can have per-scheme and/or per-resource tokens, and we need to store additional metadata as well. By exposing this as a service, we can reuse the logic elsewhere (I needed it in the BFF to use its session store as a token store).
@josephdecock josephdecock requested a review from brockallen April 30, 2024 04:30
@brockallen
Copy link
Member

Only review question: service interface or extension methods?

@josephdecock
Copy link
Member Author

I don't have a strong opinion on service vs extension method.

Pro/con of service:
Con: Service suggests an extensibility point where none is intended
Pro: Service might be easier to mock

@brockallen
Copy link
Member

Con: Service suggests an extensibility point where none is intended

Right... this was the main thing I was worried about. Also con, if the implementation needs more stuff from DI (e.g. logger) then it makes more sense as a service.

Anyway, your call.

@josephdecock
Copy link
Member Author

We do have a lot of dependencies, so I'm leaning away from extension methods actually:

    IOptions<UserTokenManagementOptions> tokenManagementOptions, // TODO - Consider option lifetime
    IAuthenticationSchemeProvider schemeProvider,
    ILogger<StoreTokensInAuthenticationProperties> logger,
    IOptionsMonitor<CookieAuthenticationOptions> cookieOptionsMonitor

@josephdecock josephdecock merged commit 5d114aa into main May 2, 2024
5 checks passed
@josephdecock josephdecock deleted the joe/tokens-in-auth-properties branch May 2, 2024 18:45
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants